Fix NRange.GetOffsetAndLength truncation: cast to nuint instead of uint by Copilot · Pull Request #124300 · dotnet/runtime

Copilot · 2026-02-11T21:39:28Z

Description

NRange.GetOffsetAndLength casts its nint parameters to uint for range validation. On 64-bit platforms, this truncates values larger than uint.MaxValue, allowing invalid ranges to pass validation silently.

// Before: truncates upper 32 bits on 64-bit platforms
if ((uint)end > (uint)length || (uint)start > (uint)end)

// After: correct comparison for all nint values
if ((nuint)end > (nuint)length || (nuint)start > (nuint)end)

For example, start = 0x1_0000_0001 and end = 2 with length = 0x1_0000_0003: the (uint) cast truncates start to 1, making 1 > 2 false and incorrectly passing validation.

Note: Other (uint) conversion issues in NRange (e.g., ToString) are intentionally not addressed here — those are covered by #124294.

Tests

Converted existing GetOffsetAndLengthTest [Fact] to [Theory] with [InlineData]
Split success and exception cases into separate theory methods
Added [ConditionalTheory(Is64BitProcess)] tests exercising values above uint.MaxValue, including a case that directly catches the truncation bug

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

… tests Co-authored-by: vcsjones <361677+vcsjones@users.noreply.github.com>

Co-authored-by: vcsjones <361677+vcsjones@users.noreply.github.com>

dotnet-policy-service · 2026-02-11T22:06:47Z

Tagging subscribers to this area: @dotnet/area-system-numerics-tensors
See info in area-owners.md if you want to be subscribed.

Copilot

Pull request overview

Fixes incorrect range validation in System.Buffers.NRange.GetOffsetAndLength on 64-bit platforms by avoiding truncating casts, ensuring out-of-range values above uint.MaxValue are rejected correctly.

Changes:

Update GetOffsetAndLength validation logic to cast to nuint instead of uint to prevent 64-bit truncation.
Refactor GetOffsetAndLength tests from a single [Fact] into multiple [Theory] cases.
Add 64-bit-only test cases covering values above uint.MaxValue, including regression coverage for the truncation scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/libraries/System.Numerics.Tensors/src/System/Buffers/NRange.cs	Fixes range validation by using `nuint` casts to avoid 64-bit truncation.
src/libraries/System.Numerics.Tensors/tests/NRangeTests.cs	Expands/organizes test coverage and adds 64-bit regression cases for large values.

vcsjones · 2026-02-12T03:24:29Z

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR.

Please keep your review focused just to GetOffsetAndLength. Other related fixes are in #124294 and are out of scope for this review.

Copilot · 2026-02-12T03:29:16Z

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR.

Please keep your review focused just to GetOffsetAndLength. Other related fixes are in https://g...

🤖 Copilot Code Review (focused on `GetOffsetAndLength`)

Holistic Assessment

Motivation: The bug is real. NRange.GetOffsetAndLength uses (uint) casts on nint values for bounds validation. On 64-bit, nint is 8 bytes but uint is 4, so values > uint.MaxValue get silently truncated, allowing invalid ranges to pass. This is a correctness bug — the analogous Range.GetOffsetAndLength uses (uint) on int which is same-width and correct.

Approach: The fix is correct and minimal — changing (uint) to (nuint) makes it a same-width unsigned cast, exactly mirroring Range's pattern at the native-integer level.

Summary: ✅ LGTM. The 1-line production fix is correct. The tests are well-structured: the original [Fact] is properly converted to [Theory]/[InlineData], and 64-bit regression tests use [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] consistent with repo conventions. The scope is appropriately limited to GetOffsetAndLength only.

Detailed Findings

✅ Production fix — Correct `(uint)` → `(nuint)` cast

The change on line 113 from (uint) to (nuint) is the right fix. Range.GetOffsetAndLength uses (uint) on int parameters (same 4-byte width), so NRange.GetOffsetAndLength should use (nuint) on nint parameters (same native width). This preserves the idiomatic unsigned bounds-check pattern the JIT recognizes while fixing the 64-bit truncation.

✅ Scope — Correctly limited to `GetOffsetAndLength`

The ToString() method still has (uint) casts on lines 73 and 84, which is intentional — those are addressed by PR #124294. This PR stays focused.

✅ Test structure — Follows repo conventions

[Theory] + [InlineData] over individual [Fact] methods ✓
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] for 64-bit-only tests ✓
Using long parameters in [InlineData] with (nint) casts (since nint can't be an attribute constant) ✓
Separate theories for success vs. exception cases ✓

✅ Test coverage — Regression case catches the bug

The GetOffsetAndLengthThrowsTest64 case [InlineData((long)uint.MaxValue + 1, false, 2, false, (long)uint.MaxValue + 3)] is a good regression case: start = 0x1_0000_0001 and end = 2 means start > end, which should throw. With the old (uint) cast, start truncates to 1, making 1 > 2 false, so validation incorrectly passes.

Initial plan

1f0db39

Copilot AI assigned Copilot and vcsjones Feb 11, 2026

Copilot started work on behalf of vcsjones February 11, 2026 21:39 View session

github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 11, 2026

Fix NRange.GetOffsetAndLength: change (uint) casts to (nuint) and add…

3a1f199

… tests Co-authored-by: vcsjones <361677+vcsjones@users.noreply.github.com>

vcsjones added area-System.Numerics.Tensors and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Feb 11, 2026

Revert unintended ToString() changes - only fix GetOffsetAndLength

7314fec

Co-authored-by: vcsjones <361677+vcsjones@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix bug in NRange.GetOffsetAndLength parameter conversion~~ Fix NRange.GetOffsetAndLength truncation: cast to nuint instead of uint Feb 11, 2026

Copilot AI requested a review from vcsjones February 11, 2026 22:07

Copilot finished work on behalf of vcsjones February 11, 2026 22:07

vcsjones marked this pull request as ready for review February 11, 2026 22:09

Copilot AI review requested due to automatic review settings February 11, 2026 22:09

Copilot started reviewing on behalf of vcsjones February 11, 2026 22:10 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

Copilot started work on behalf of vcsjones February 12, 2026 03:24 View session

Copilot finished work on behalf of vcsjones February 12, 2026 03:29

This was referenced Feb 12, 2026

[wasm] OpenQA.Selenium.WebDriverTimeoutException: timeout: Timed out receiving message from renderer #117486

Open

System.Security.Cryptography.CryptographicException : m_safeCertContext is an invalid handle. #124279

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix NRange.GetOffsetAndLength truncation: cast to nuint instead of uint#124300

Fix NRange.GetOffsetAndLength truncation: cast to nuint instead of uint#124300
Copilot wants to merge 3 commits intomainfrom
copilot/fix-nrange-getoffsetandlength-bug

Copilot AI commented Feb 11, 2026 •

edited

Loading

Uh oh!

dotnet-policy-service bot commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

vcsjones commented Feb 12, 2026

Uh oh!

Copilot AI commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Copilot AI commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Tests

Uh oh!

dotnet-policy-service bot commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

vcsjones commented Feb 12, 2026

Uh oh!

Copilot AI commented Feb 12, 2026

🤖 Copilot Code Review (focused on GetOffsetAndLength)

Holistic Assessment

Detailed Findings

✅ Production fix — Correct (uint) → (nuint) cast

✅ Scope — Correctly limited to GetOffsetAndLength

✅ Test structure — Follows repo conventions

✅ Test coverage — Regression case catches the bug

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Feb 11, 2026 •

edited

Loading

🤖 Copilot Code Review (focused on `GetOffsetAndLength`)

✅ Production fix — Correct `(uint)` → `(nuint)` cast

✅ Scope — Correctly limited to `GetOffsetAndLength`